-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add basic PWA support #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
PascalRepond
commented
Jan 15, 2026
- Also updates dependencies to address security vulnerabilities.
📝 WalkthroughWalkthroughAdds PWA support: a service worker file, client registration, a Django view to serve the worker at /service-worker.js, a web app manifest, and manifest/theme-color references in the base template; plus minor responsive/style tweaks in templates and CSS. Changes
Sequence DiagramsequenceDiagram
actor Browser
participant DOM as "DOMContentLoaded Handler"
participant SWAPI as "ServiceWorker API"
participant Django as "Django View (/service-worker.js)"
participant SW as "Service Worker"
participant Cache as "Cache API"
Browser->>DOM: Page loaded
DOM->>SWAPI: check support
alt supported
DOM->>SWAPI: register("/service-worker.js")
SWAPI->>Django: GET /service-worker.js
Django-->>SWAPI: service-worker.js (JS)
SWAPI->>SW: install event
SW->>Cache: cache STATIC_ASSETS (CACHE_NAME)
SW->>SWAPI: skipWaiting()
SWAPI->>SW: activate event
SW->>Cache: delete old caches
SW->>SWAPI: clients.claim()
else not supported
DOM->>Browser: log unsupported
end
rect rgba(220,150,100,0.5)
Note over Browser,SW: Subsequent fetches
Browser->>SW: request
alt Accept: text/html
SW->>Browser: try network
alt network ok
SW->>Cache: store HTML response
SW->>Browser: return response
else network fail
SW->>Cache: match cached HTML
SW->>Browser: return cached response
end
else Request path starts with /static/
SW->>Cache: match asset
alt cache hit
SW->>Browser: return cached asset
else cache miss
SW->>Browser: fetch asset
SW->>Cache: store asset
end
else non-GET or cross-origin
SW->>Browser: bypass (fetch network)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧠 Learnings (5)📚 Learning: 2025-12-26T15:18:46.932ZApplied to files:
📚 Learning: 2026-01-03T21:16:52.649ZApplied to files:
📚 Learning: 2026-01-04T08:55:43.091ZApplied to files:
📚 Learning: 2026-01-02T14:50:26.110ZApplied to files:
📚 Learning: 2025-12-27T18:00:03.595ZApplied to files:
🔇 Additional comments (5)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/config/urls.py`:
- Around line 9-13: The service_worker view currently opens sw_path directly and
will raise FileNotFoundError; update service_worker to catch FileNotFoundError
(and optionally PermissionError) around sw_path.open() and return an appropriate
404 response (e.g., HttpResponseNotFound or HttpResponse with status=404 and a
clear message) instead of letting a 500 occur; keep using settings.BASE_DIR and
sw_path for locating the file and still return HttpResponse(f.read(),
content_type="application/javascript") on success.
In `@src/static/js/service-worker.js`:
- Around line 76-81: The service-worker is caching all fetch responses
(including 404s/errors) in the fetch handler; update the
fetch(request).then((response) => { ... }) logic to only clone and cache the
response when response.ok is true (i.e., successful HTTP 2xx) and skip cache.put
otherwise, using CACHE_NAME; ensure you still return the original response to
the caller and only call response.clone() when you will cache it.
- Around line 52-59: The fetch response handler currently caches every fetched
response (see fetch(request) -> then(response) using responseClone and
caches.open(CACHE_NAME)). Modify that handler to check response.ok (or
response.status < 400) before cloning and calling cache.put(request,
responseClone) so only successful responses are cached; keep the existing return
response behavior unchanged.
🧹 Nitpick comments (1)
src/static/manifest.json (1)
9-22: Consider adding maskable icon support for better Android compatibility.The icons specify
"purpose": "any". For optimal display on Android devices using adaptive icons, consider adding maskable variants or updating to"any maskable"if the icons include sufficient padding for the safe zone.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
src/static/images/icons/icon-192.pngis excluded by!**/*.pngsrc/static/images/icons/icon-512.pngis excluded by!**/*.pngsrc/theme/static_src/package-lock.jsonis excluded by!**/package-lock.jsonuv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
src/config/urls.pysrc/static/js/base.jssrc/static/js/service-worker.jssrc/static/manifest.jsonsrc/templates/base/base.html
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-01-02T14:50:26.110Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 32
File: src/config/urls.py:21-31
Timestamp: 2026-01-02T14:50:26.110Z
Learning: In Django projects, avoid using django.views.static.serve in production (i.e., when DEBUG is False). Review any urls.py configurations that route static or media files through this view and replace with a proper static file server (e.g., nginx/Apache) or use a dedicated tool like WhiteNoise for production-ready static file handling. This guidance applies to all Django url configurations, not just a single file.
Applied to files:
src/config/urls.py
📚 Learning: 2025-12-26T15:18:46.932Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 21
File: src/templates/accounts/profile_edit.html:23-58
Timestamp: 2025-12-26T15:18:46.932Z
Learning: In Django projects, attributes added to a form field's widget via field.widget.attrs.update(...) in the form's __init__ are rendered when using {{ form.field }} in templates. No explicit attribute definitions are needed in the template. This applies to templates under src/templates in Django apps; ensure you update attrs in __init__ for consistent HTMX behavior.
Applied to files:
src/templates/base/base.html
📚 Learning: 2026-01-03T21:16:52.649Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-items.html:67-67
Timestamp: 2026-01-03T21:16:52.649Z
Learning: In HTML templates (e.g., src/templates/partials/media-items.html), Tailwind CSS v4.1.11 supports arbitrary numeric utilities (max-w-{n}, w-{n}, h-{n}) that map to calc(var(--spacing) * n). These should be considered valid Tailwind classes and not flagged as invalid. Ensure review tooling and linters treat such numeric utilities as allowed, and apply this guidance to other HTML/template files in the repository.
Applied to files:
src/templates/base/base.html
📚 Learning: 2026-01-04T08:55:43.091Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-score-badge.html:1-9
Timestamp: 2026-01-04T08:55:43.091Z
Learning: In Django projects, configuring heroicons in TEMPLATES builtins makes heroicons.templatetags.heroicons available in all templates without needing {% load heroicons %}. For template reviews in this codebase, assume heroicon tags work globally in HTML templates under templates/, so avoid requiring explicit loads. If a template still uses {% load heroicons %}, assess whether the load is unnecessary and can be removed; ensure no conflicts arise from global tags.
Applied to files:
src/templates/base/base.html
🔇 Additional comments (5)
src/config/urls.py (1)
31-41: Production media serving is documented with appropriate caveats.The comment appropriately documents the trade-off. Based on learnings,
django.views.static.serveshould generally be avoided in production, but the inline documentation here acknowledges the limitations and suggests alternatives for high-traffic scenarios. This is acceptable for the stated use case.src/static/js/base.js (1)
140-151: LGTM!The service worker registration is well-implemented with proper feature detection and error handling. The registration path correctly matches the Django URL pattern.
src/templates/base/base.html (1)
21-22: LGTM!The manifest link and theme-color meta tag are correctly added and consistent with the manifest.json values.
src/static/js/service-worker.js (2)
10-18: Install event implementation looks good.The install handler correctly caches static assets and uses
skipWaiting()to activate immediately. The implementation is appropriate for a basic PWA.
20-32: Activate event is well-implemented.The cache cleanup logic correctly removes old caches and
clients.claim()ensures the new service worker takes control immediately.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Enhances the responsivity of the app (login form, media list). - Also updates dependencies to address security vulnerabilities.